Skip to content

Implement GNO head#37

Draft
aprokop wants to merge 22 commits intoORNL:mainfrom
aprokop:gno_head
Draft

Implement GNO head#37
aprokop wants to merge 22 commits intoORNL:mainfrom
aprokop:gno_head

Conversation

@aprokop
Copy link
Copy Markdown
Collaborator

@aprokop aprokop commented Feb 25, 2026

No description provided.

Copy link
Copy Markdown
Collaborator

@pzhanggit pzhanggit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the detailed comments. I might be missing something but I do not see how geometry info is fed into the model. Could you explain?


def forward(self, y, x, f_y, key):
if f_y is not None:
if f_y.ndim == 3 and f_y.shape[0] == -1:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the f_y.shape[0] == -1 mean?

return_dict = self.search_fn(data, queries, radius, self.return_norm)
return return_dict

def custom_neighbor_search(data: torch.Tensor, queries: torch.Tensor, radius: float, return_norm: bool=False):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to read this part a bit more to understand it. Did you check if returned neighbors make sense?

Comment on lines +453 to +466
in_channels: int,
out_channels: int,
coord_dim: int,
radius: float,
transform_type="linear",
weighting_fn: Optional[Callable]=None,
reduction: Literal['sum', 'mean']='sum',
pos_embedding_type: str='transformer',
pos_embedding_channels: int=32,
pos_embedding_max_positions: int=10000,
channel_mlp_layers: List[int]=[128,256,128],
channel_mlp_non_linearity=F.gelu,
channel_mlp: nn.Module=None,
use_torch_scatter_reduce: bool=True):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need all the arguments? Wondering if we should remove the arguments and functions that we do not need for simplicity.


# FIXME: should there be a normalization layer here

self.res = params["resolution"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add: so the ratio between D,H,W and params["resolution"] would be the equivalent patch size? If so, should we use patch size as input?

@aprokop
Copy link
Copy Markdown
Collaborator Author

aprokop commented Feb 26, 2026

See the detailed comments. I might be missing something but I do not see how geometry info is fed into the model. Could you explain?

I'll answer as to how I understand your question.

The Flow3D dataset returns a dictionary with geometry and geometry id, which is then unpacked in train.py and then packed into (x, geometry). Whenever geometry is present, we are going through GNO head, where we first use geometry's bounding box to scale and translate the latent geometry (to cover the same area), and then to do the neighbor search that will be fed to the neuraloperator's GNO block. The operation is then reversed in debedding.

@pzhanggit
Copy link
Copy Markdown
Collaborator

See the detailed comments. I might be missing something but I do not see how geometry info is fed into the model. Could you explain?

I'll answer as to how I understand your question.

The Flow3D dataset returns a dictionary with geometry and geometry id, which is then unpacked in train.py and then packed into (x, geometry). Whenever geometry is present, we are going through GNO head, where we first use geometry's bounding box to scale and translate the latent geometry (to cover the same area), and then to do the neighbor search that will be fed to the neuraloperator's GNO block. The operation is then reversed in debedding.

Thanks, Andrey. I understand what you did in the PR, but the part confuses me is: what do you mean by geometry. I was expecting a tensor to show the sold and fluid region, but so far the geometry seems to be just grid coordinates/indices?

radius_in: 1.8
radius_out: 1.9
# resolution: [48, 48, 48]
resolution: [16, 16, 16]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolution and patch_size


self.neighbors_dict = {}

def forward(self, y, x, f_y, key):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the forward function modified from the original GNOBlock or we can use the original one?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is modified by using neighbor caching (using neighbors_dict). The diff is like this:
Screenshot 2026-02-28 at 3 20 18 PM

I think the first if statement was in one of the previous neuraloperator versions and can be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants